Could you do a full replay and measure the performance penalty of your changes? If it is insignificant I'd tend to agree with you - simplicity over performance. OTOH, if it is significant we should try to optimize.

Actually I've done a full replay, but didn't record the time used. Will do again.

@pmconrad looks like the patch doesn't affect replay at all, perhaps due to no signature verification while replaying?

• With the whole patch:

222740ms th_a db_management.cpp:59 reindex ] reindexing blockchain
222740ms th_a db_management.cpp:65 reindex ] Replaying blocks, starting at 1...
2433130ms th_a db_management.cpp:78 reindex ] Writing database to disk at block 25162303
2441179ms th_a db_management.cpp:80 reindex ] Done
2454500ms th_a db_management.cpp:122 reindex ] Done reindexing, elapsed time: 5831.76034100000015314 sec

• Without the patch:

1200426ms th_a db_management.cpp:59 reindex ] reindexing blockchain
1200426ms th_a db_management.cpp:65 reindex ] Replaying blocks, starting at 1...
3530797ms th_a db_management.cpp:78 reindex ] Writing database to disk at block 25165591
3536869ms th_a db_management.cpp:80 reindex ] Done
3552218ms th_a db_management.cpp:122 reindex ] Done reindexing, elapsed time: 5951.79204399999980524 sec

In above data, the latter spent even more time, but perhaps it is caused by processing the 2288 more blocks. By the way, I did enable grouped_orders plugin while replaying, so time of both is a bit longer than usual.

//Update:
No much difference after disabled grouped_orders plugin:

1852293ms th_a db_management.cpp:59 reindex ] reindexing blockchain
1852293ms th_a db_management.cpp:65 reindex ] Replaying blocks, starting at 1...
235934ms th_a db_management.cpp:78 reindex ] Writing database to disk at block 25180414
241954ms th_a db_management.cpp:80 reindex ] Done
255375ms th_a db_management.cpp:122 reindex ] Done reindexing, elapsed time: 5603.08208300000023883 sec

Thanks. That really is insignificant. Interesting.
Signature verification shouldn't matter to the question at hand. Blocks are read from disk, deserialized and processed. I'd assumed that the deserialization would be slower than before, but apparently I was wrong.

@pmconrad I got some time, so are making changes to improve performance. Will push Soon. Please check again later.

Result of replay after the "optimization" commit (with frustration):

1064463ms th_a db_management.cpp:59 reindex ] reindexing blockchain
1064463ms th_a db_management.cpp:65 reindex ] Replaying blocks, starting at 1...
3429348ms th_a db_management.cpp:78 reindex ] Writing database to disk at block 25186907
3437237ms th_a db_management.cpp:80 reindex ] Done
3451642ms th_a db_management.cpp:122 reindex ] Done reindexing, elapsed time: 5987.17886300000009214 sec

Lol